-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(files_versions): Refactor function for lisibility #48325
Conversation
Signed-off-by: Git'Fellow <12234510+solracsf@users.noreply.github.com> fix: typo
38a7374
to
9ef2721
Compare
I don't consider that an improvement 🙈 Fine by me, but please fix the commit message ;) |
Sure, opinions. What's wrong with the commit message? 🤔 |
Removed my assignment as this is php code style related which I would pass to @come-nc and @provokateurin for files backend |
The commit type should be lisbility => legibility or readability |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with @kesselb, it's not really better than before.
It also abuses the match functionality, so I don't really like that either.
@@ -331,7 +302,7 @@ public function pre_renameOrCopy_hook(Node $source, Node $target): void { | |||
$manager = Filesystem::getMountManager(); | |||
$mount = $manager->find($absOldPath); | |||
$internalPath = $mount->getInternalPath($absOldPath); | |||
if ($internalPath === '' and $mount instanceof MoveableMount) { | |||
if ($internalPath === '' && $mount instanceof MoveableMount) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should have a cs fixer rule for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rector does this as well, I've never seen anyone use this syntax in recent years so it is only in legacy code anyway.
I agree with the others, I dislike using match like this. It’s harder to read as the calls are not aligned with each other anymore. |
Checklist